Yeh - needs a little more work. I think

@sei-vsarvepalli

Please take a look. I tried to incorporate the fix in the code and clean it up a little but I'm left with 2 failing tests, which also fail on your branch. Need some helping fixing those tests (or implementation). then happy to merge.

See my https://github.com/sei-vsarvepalli/expr-eval-secure/tree/member-access branch that should fix all the tests and the more security problems that were found from using __expr_eval_safe_def property that can be object defined and NOT trusted.

Please test, fix any README's, indentation etc. and release at your convenience.

Yeh - needs a little more work. I think

@sei-vsarvepalli
Please take a look. I tried to incorporate the fix in the code and clean it up a little but I'm left with 2 failing tests, which also fail on your branch. Need some helping fixing those tests (or implementation). then happy to merge.

See my https://github.com/sei-vsarvepalli/expr-eval-secure/tree/member-access branch that should fix all the tests and the more security problems that were found from using __expr_eval_safe_def property that can be object defined and NOT trusted.

Please test, fix any README's, indentation etc. and release at your convenience.

hello @jorenbroekema

Any questions? or any feedback you need?

Hi @sei-vsarvepalli, while I appreciate the efforts of this fork, I am creating an actively maintained, community-driven fork of expr-eval to address this fix and future ones. I have also been awaiting this security patch and figured to move forward with a fork of my own. 🙂

If you would like, please open a pull request addressing this change, which you have done here, or I will go ahead and make the change by the end of the day tomorrow. expr-eval-community

Thank you!

Hi @sei-vsarvepalli, while I appreciate the efforts of this fork, I am creating an actively maintained, community-driven fork of expr-eval to address this fix and future ones. I have also been awaiting this security patch and figured to move forward with a fork of my own. 🙂

If you would like, please open a pull request addressing this change, which you have done here, or I will go ahead and make the change by the end of the day tomorrow. expr-eval-community

Thank you!

Or you could raise a PR to this branch to fix it with @sei-vsarvepalli tips, also happy to invite you as a collaborator to this one if that helps. I think more forks will just increase friction right?

I just haven't found the time (unemployed, looking for a new job, christmas holidays coming up etc.) to wrap this up myself but happy to get help on it.

No worries at all! I completely understand where you're coming from, and I hope my comment didn't come across the wrong way! Fewer forks would definitely make things smoother. I have seen a few forks of this package now due to the security issues that have arisen - @jorenbroekema

If we could get this PR merged, that would be fantastic: jorenbroekema#3.

@Mwpereira no worries, I know you mean well and I get that folks want to get this fixed rather sooner than later :)

I had some time today to open jorenbroekema#4 which combines jorenbroekema#3 + my minor improvements + version bump and changelog entry, I had one question about a small code change that I'm not sure we still need, besides that we can merge and release it to NPM I think.

I'd like to get at least you and @sei-vsarvepalli as contributors to expr-eval-fork and set up CI/CD so we can move things along faster next time with at least the 3 of us and not be blocked by my personal schedule and/or lack of time. I appreciate the idea of making it more of a community fork 😅 I'd rather not maintain it alone hehe